Fix: origin-rewriting wrappers bypass high-security restrictions by n13 · Pull Request #610 · Quantus-Network/chain

n13 · 2026-06-29T07:10:31Z

Summary

High-security (HS) accounts are meant to be limited to whitelisted calls (e.g. reversible schedule_transfer/cancel). They could be bypassed: Recovery::as_recovered and Utility::as_derivative synthesize a fresh Signed origin after top-level transaction validation, so a non-HS outer signer could dispatch a non-whitelisted call as an HS account — including when nested under batch/batch_all/force_batch/if_else.

This PR enforces the HS whitelist at the effective dispatched origin, entirely at the runtime/extension layer:

HighSecurityConfig::call_allowed_for recursively resolves origin-rewriting wrappers (as_recovered/as_derivative) and origin-preserving combinators (batch*/if_else), re-checking the whitelist at each effective signer. Root-only dispatch_as/with_weight and the scheduler are intentionally not traversed.
Shared via a new HighSecurityInspector::is_call_allowed provided method, so ReversibleTransactionExtension and pallet-multisig use one implementation (no duplicated logic).
Recursion is bounded and fail-closed (MAX_CALL_DEPTH = 16, well below MAX_EXTRINSIC_DEPTH = 256): over-nested calls are rejected rather than allowed to escape the whitelist, so validation work can't be exhausted.
The extension weight() scales with the number of nodes traversed (one is_high_security read per node).
pallet-recovery and pallet-utility are left unchanged from upstream Substrate.

Test plan

cargo test -p quantus-runtime --lib transaction_extensions::tests — origin-rewriter bypass blocked, whitelisted inner call allowed, nested-in-batch bypass blocked, non-HS origin unaffected, over-nested call rejected fail-closed
cargo test -p pallet-multisig
cargo test -p pallet-reversible-transfers
Runtime integration tests (recovery / reversible)

Note

High Risk
This is a security fix for account restriction enforcement; incorrect recursion or depth handling could still allow HS bypass or reject valid transactions.

Overview
Closes a bypass where high-security (HS) call limits could be evaded by wrapping disallowed inner calls in Recovery::as_recovered or Utility::as_derivative (and batch* / if_else), because those pallets rewrite the signed origin after top-level validation.

HighSecurityInspector::is_call_allowed is added with a default that only checks the top-level call; the runtime overrides it via HighSecurityConfig::call_allowed_for, which recursively walks origin-rewriting and batching wrappers, re-applies the HS whitelist at each effective signer, and rejects over-deep nesting (MAX_CALL_DEPTH = 16, fail-closed). ReversibleTransactionExtension now uses this helper instead of a shallow HS + whitelist check, and its weight() scales with high_security_read_count. pallet-multisig execute re-checks with is_call_allowed instead of only testing the outer call against the whitelist.

Runtime tests cover blocked/allowed paths for as_recovered, as_derivative, batch-wrapped bypasses, and excessive nesting.

^{Reviewed by Cursor Bugbot for commit 32c521a. Configure here.}

for all origin-altering calls

Bound `HighSecurityConfig::call_allowed_for` to a fixed nesting depth (MAX_CALL_DEPTH = 16) and fail closed beyond it, so deeply nested origin-rewriting/combinator calls cannot exhaust transaction-validation work. Adds a regression test.

n13

Verdict: Approve — no remaining blockers from this review.

Update: the only blocker from my earlier review (the failing Format check) is resolved. Commit e196524a ("format") makes the Fast Checks (Format) job pass, and git show -w confirms it is whitespace-only (rustfmt line-reflow in runtime/src/transaction_extensions.rs — weight(), validate_with(), and two test constructors), so the logic and tests validated earlier are unaffected. (Build & Test / Clippy & Doc were still running at review time — merge once they go green.)

Effective-origin high-security enforcement (reconfirmed):

The recursive resolver re-checks the whitelist at the effective dispatched origin, covering the origin-rewriting wrappers (Recovery::as_recovered, Utility::as_derivative) and the origin-preserving combinators (batch / batch_all / force_batch / if_else).
The remaining pallet-utility origin-altering calls — dispatch_as, with_weight, dispatch_as_fallible — are all ensure_root, so they are not an unprivileged bypass and correctly are not traversed. The scheduler's extrinsics are disabled.
ReversibleTransactionExtension and pallet-multisig share the single HighSecurityInspector::is_call_allowed implementation (no duplicated logic); multisig re-checks at execute time against the multisig address.
Recursion is bounded (MAX_CALL_DEPTH = 16) and fails closed, well under the MAX_EXTRINSIC_DEPTH = 256 decode limit; weight() scales with the number of traversed nodes.
Recovery address handling is safe: this runtime sets type Lookup = AccountIdLookup, which only resolves MultiAddress::Id, so the non-Id arm cannot dispatch and is not an alternate-encoding bypass.

No security or logic blocker found.

Local tests (still valid — the format commit is whitespace-only):

cargo test -p quantus-runtime --lib transaction_extensions::tests passed
cargo test -p pallet-multisig passed
cargo test -p pallet-reversible-transfers passed

n13

Updated verdict after the format commit: previous blocker cleared.

The new format commit fixed the issue I flagged; GitHub's Fast Checks (Format) job is now passing. My code-review verdict is approve/merge-ready from the effective-origin high-security enforcement perspective.

Remaining note: the build/test matrix and clippy/doc jobs are still running on the latest commit, so I would wait for those checks to finish green before merging.

cursor

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 32c521a. Configure here.}

cursor · 2026-06-30T08:38:21Z

+						Self::call_allowed_for(target, call, depth + 1),
+					// Other address kinds are unresolvable by the runtime lookup and cannot
+					// dispatch, so there is no effective origin to enforce.
+					_ => true,


Recovery non-Id skips HS check

High Severity

In call_allowed_for, Recovery::as_recovered only recurses into the inner call when account is MultiAddress::Id. Other lookup forms return true without inspecting the inner call. as_recovered resolves accounts via T::Lookup::lookup at dispatch, so a non-whitelisted inner call for a high-security account can pass validation and still execute.

^{Reviewed by Cursor Bugbot for commit 32c521a. Configure here.}

cursor · 2026-06-30T08:38:21Z

-			if T::HighSecurity::is_high_security(&multisig_address) &&
-				!T::HighSecurity::is_whitelisted(&call)
-			{
+			if !T::HighSecurity::is_call_allowed(&multisig_address, &call) {


Propose skips recursive HS check

Medium Severity

propose still gates high-security multisigs with a shallow is_high_security plus is_whitelisted check on the decoded call, while execute now uses is_call_allowed, which applies the depth limit and recursive origin resolution. Proposals can be created and approved whose inner calls execute will always reject.

Additional Locations (1)

pallets/multisig/src/lib.rs#L1127-L1135

^{Reviewed by Cursor Bugbot for commit 32c521a. Configure here.}

cursor · 2026-06-30T08:38:21Z

+			_ => 0,
+		};
+		inner.saturating_add(1)
+	}


Weight counts unbounded call depth

Medium Severity

high_security_read_count walks the full nested call tree with no MAX_CALL_DEPTH cap, while call_allowed_for stops rejecting after depth 16. ReversibleTransactionExtension::weight uses this count for every signed extrinsic, so pathological nesting up to the decode limit can force large recursive work during fee estimation even when validation fails early on depth.

^{Reviewed by Cursor Bugbot for commit 32c521a. Configure here.}

n13 added 2 commits June 29, 2026 14:50

recursive check for whitelisted calls

96fc033

for all origin-altering calls

cap high-security call resolver recursion depth

6ae322e

Bound `HighSecurityConfig::call_allowed_for` to a fixed nesting depth (MAX_CALL_DEPTH = 16) and fail closed beyond it, so deeply nested origin-rewriting/combinator calls cannot exhaust transaction-validation work. Adds a regression test.

n13 commented Jun 29, 2026

View reviewed changes

format

e196524

n13 commented Jun 29, 2026

View reviewed changes

Merge branch 'main' into high_security_fix

32c521a

cursor Bot reviewed Jun 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix: origin-rewriting wrappers bypass high-security restrictions#610

Fix: origin-rewriting wrappers bypass high-security restrictions#610
n13 wants to merge 4 commits into
mainfrom
high_security_fix

n13 commented Jun 29, 2026 •

edited by cursor Bot

Loading

Uh oh!

n13 left a comment •

edited

Loading

Uh oh!

n13 left a comment

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Jun 30, 2026

Uh oh!

cursor Bot Jun 30, 2026

Uh oh!

cursor Bot Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

n13 commented Jun 29, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

n13 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

n13 left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jun 30, 2026

Choose a reason for hiding this comment

Recovery non-Id skips HS check

Uh oh!

cursor Bot Jun 30, 2026

Choose a reason for hiding this comment

Propose skips recursive HS check

Uh oh!

cursor Bot Jun 30, 2026

Choose a reason for hiding this comment

Weight counts unbounded call depth

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

n13 commented Jun 29, 2026 •

edited by cursor Bot

Loading

n13 left a comment •

edited

Loading